-
Notifications
You must be signed in to change notification settings - Fork 39
fix: removed support for experimental loader flag #2153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
66f40a8 to
0b41e6e
Compare
af69d60 to
70826e1
Compare
|
We need a todo to cleanup task for internal Github usages of "--experimental-loader". |
| // eslint-disable-next-line max-len | ||
| `The package @instana/aws-fargate requires at least Node.js ${nodeJsVersionCheck.minimumNodeJsVersion} but this process is ` + | ||
| `running on Node.js ${process.version}. This Fargate container will not be monitored by Instana.` + | ||
| 'See https://www.ibm.com/docs/en/instana-observability/current?topic=agents-aws-fargate#versioning.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.ibm.com/docs/en/instana-observability/current?topic=agents-aws-fargate#versioning
Is this still a valid link? 🤔
If not, its probably good to fix that straight on main branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it does, but somehow I cant follow we link there specifically 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this link and add the v4-v5 migration link once its available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a TODO in v5
| console.error( | ||
| `The package @instana/aws-fargate requires at least Node.js ${minimumNodeJsVersion} but this process is ` + | ||
| // eslint-disable-next-line max-len | ||
| `The package @instana/aws-fargate requires at least Node.js ${nodeJsVersionCheck.minimumNodeJsVersion} but this process is ` + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a customer uses < 18.19 and the legacy esm loader, he will first run into this warning. Then he updates the node version, restarts and then he runs into the next warning.
I am wondering if we should already link to the migration? 🤔 Because we currently only link to agents-aws-fargate#versioning.
kirrg001
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-approving
Co-authored-by: kirrg001 <[email protected]>
BREAKING CHANGE
Notes:
--experimental-loaderis already not working for Node versions >= v18.19.0 (documented already)esm-loader.mjsworks with--experimental-loaderonly.We are dropping support for < v18.19.
Reference
Autotrace webhook: https://github.ibm.com/instana/autotrace-mutating-webhook/pull/670